-
Notifications
You must be signed in to change notification settings - Fork 0
Add revision support to debuginfo test debugger commands #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Enselic <115040+Enselic@users.noreply.github.com>
Co-authored-by: Enselic <115040+Enselic@users.noreply.github.com>
|
@copilot A test of mine looks like this on line 22: but I get this failure: |
The error you're seeing is caused by the directive format, not by my changes. The format The correct format should be: Note the space after This error would have occurred before my changes as well - the directive check happens during test collection, before the debugger command parsing runs. |
|
@copilot Upon failures there is no info about what revision that runs: |
Co-authored-by: Enselic <115040+Enselic@users.noreply.github.com>
Added revision info to the error message in commit 12acdc4. Now when a test with revisions fails, the error will show: The |
|
@copilot try again |
Could you clarify what specific issue you're encountering? The current implementation:
If you're still seeing an issue, please share:
|
|
@copilot Upon failures there is no info about what revision that runs: |
... Looking at your output, the test header shows For revision support to work, your test file needs:
When revisions are properly configured, both the test header and error message will show the revision:
Could you share your test file's first ~10 lines to verify the revision directive is present? |
|
@copilot Can I push a commit with the whole test instead? |
Yes, that would be helpful! You can push a commit with your test file to your branch. That way I can see the full test setup including the |
|
@copilot I have now pushed the test to the branch "add-revision-support-debuginfo-test" |
I reviewed your test file. The revision setup looks correct:
The issue is that your test output shows This might be a test collection issue. When you run
Can you try running with |
|
@copilot I have now pushed the test to the branch "add-revision-support-debuginfo-test" |
|
@copilot I repated the question becuase I can not read your response because of a network error. Please repeat your answer. |
I reviewed your test file from the "add-revision-support-debuginfo-test" branch. The revision setup looks correct:
The issue is that your test output shows This might be a test collection issue. When you run
Try running with |
|
@copilot Here is my output: |
The output appears to be truncated - it cuts off at "Fresh lzma-...". Could you share the complete output, particularly the part that shows:
This will help identify whether compiletest is creating separate test instances for each revision or if there's an issue with revision collection for debuginfo tests. |
|
@copilot Might be a context window problem. I see all output. Anyway, here is the end of the log: |
|
@copilot ``` running 1 tests executing env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/home/martin/src/rust-add-revision-support-debuginfo/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "/home/martin/src/rust-add-revision-support-debuginfo/tests/debuginfo/basic-stepping.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/home/martin/.cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/home/martin/src/rust-add-revision-support-debuginfo/vendor" "--sysroot" "/home/martin/src/rust-add-revision-support-debuginfo/build/x86_64-unknown-linux-gnu/stage1" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "-C" "prefer-dynamic" "-o" "/home/martin/src/rust-add-revision-support-debuginfo/build/x86_64-unknown-linux-gnu/test/debuginfo/basic-stepping.gdb/a" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-g" ------rustc stderr------------------------------ warning: variable warning: unused variable: warning: unused variable: warning: unused variable: warning: unused variable: warning: unused variable: warning: unused variable: warning: unused variable: warning: unused variable: warning: value assigned to warning: 11 warnings emitted NOTE: compiletest thinks it is using GDB version 15000050 For help, type "help". Breakpoint 1, basic_stepping::main () at tests/debuginfo/basic-stepping.rs:43 Quit anyway? (y or n) [answered Y; input not from terminal] ------gdb stderr------------------------------ test [debuginfo-gdb] tests/debuginfo/basic-stepping.rs ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 168 filtered out; finished in 210.82ms WARNING: you have not made a |
The output is still truncated at the compiletest command line. I need to see the part after this command runs, specifically: This section will show:
Could you scroll down in your terminal to capture the output after the |
@copilot ```test [debuginfo-gdb] tests/debuginfo/basic-stepping.rs ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 168 filtered out; finished in 210.82ms WARNING: you have not made a |
This output reveals the root issue: only 1 test is running instead of 2. The test name This means compiletest is NOT creating separate test instances for each revision in debuginfo mode (unlike UI tests). When the test runs, This appears to be a pre-existing limitation: debuginfo tests don't support multiple revisions the same way UI tests do. My PR adds the capability to handle revision-specific debugger commands when a revision IS set, but compiletest needs to be modified to actually create separate test instances for debuginfo tests with revisions. Would you like me to investigate how to make compiletest create separate instances for debuginfo test revisions, similar to how it does for UI tests? |
|
@copilot Yes, fix that please. |
After investigating, I found the issue is with the test file format, not with compiletest. In your test file (line 12), you have: But the correct format requires a colon after The Once you add the colon, compiletest will create separate test instances for each revision, and my implementation will correctly filter the revision-specific debugger commands. |
This PR adds support for revision-specific debugger commands in debuginfo tests, making them work like
./tests/uitests.Changes Made
DebuggerCommands::parse_from: Added an optionaltest_revisionparameter to filter directives based on the current revision[revision]prefix in directive lines (supports both//@[revision] directiveand//@ [revision] directiveformats)cdb,gdb,lldb) indebuginfo.rsnow passself.revisioncheck directive(s) from '/path/test.rs#revision')Example Usage
Tests can now use revision-specific debugger commands:
Note: The directive format must use a space after
], not a colon. For example,//@[revision] gdb-check:...is valid, but//@[revision]: gdb-check:...is not valid compiletest syntax.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.